restore: refactor split code for two phase split. #48244

3pointer · 2023-11-02T10:34:30Z

What problem does this PR solve?

Issue Number: ref #33937

Problem Summary:
split too many regions on the same node. will become bottleneck of a large scale cluster restoration.

What is changed and how it works?

introduce 2-phase split during restore. when enable fine-grained restore. it will pick keys by store count, and doing a two-phase split scatter. which will save time from scatter-heavy workloads.
refactor code and unified utils.retry for all related backoffer.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

without this change:

with this change:

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

restore: improve performance by reduce scatter operations.

ti-chi-bot · 2023-11-02T10:34:33Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

tiprow · 2023-11-02T10:34:46Z

Hi @3pointer. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

3pointer · 2023-11-03T04:10:42Z

/ok-to-test

3pointer · 2023-12-01T10:19:02Z

/ok-to-test

codecov · 2023-12-04T04:12:50Z

Codecov Report

Merging #48244 (9859de1) into master (1f49f2d) will decrease coverage by 3.2356%.
Report is 3 commits behind head on master.
The diff coverage is 0.0000%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #48244        +/-   ##
================================================
- Coverage   71.9200%   68.6844%   -3.2356%     
================================================
  Files          1438       1440         +2     
  Lines        345755     408072     +62317     
================================================
+ Hits         248667     280282     +31615     
- Misses        76821     108767     +31946     
+ Partials      20267      19023      -1244

Flag	Coverage Δ
integration	`43.9460% <0.0000%> (?)`
unit	`71.9336% <ø> (+0.0136%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`56.3129% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`43.7210% <0.0000%> (-7.8741%)`	⬇️

sre-bot · 2023-12-18T10:21:02Z

All committers have signed the CLA.

br/pkg/restore/pipeline_items.go

Leavrth · 2024-01-03T01:59:12Z

br/pkg/restore/pipeline_items.go

+	}
+
+	for _, dr := range res {
+		outCh <- dr


use ctx to avoid getting stuck

Leavrth · 2024-01-03T02:01:51Z

br/pkg/task/restore.go

@@ -942,11 +959,13 @@ func runRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf
 		})
 	}

+	blockTableStream := GoBlockCreateTablesPipeline(ctx, maxRestoreBatchSizeLimit, tableStream)


block only if granularity == string(CoarseGrained)

br/pkg/task/restore.go

Leavrth · 2024-01-03T02:45:35Z

br/pkg/task/restore.go

+		})
+
+		for _, tbl := range cachedTables {
+			outCh <- tbl


use ctx to avoid getting stuck

Leavrth · 2024-01-03T02:49:37Z

br/pkg/task/stream.go

@@ -1490,7 +1490,7 @@ func createRestoreClient(ctx context.Context, g glue.Glue, cfg *RestoreConfig, m
 	client.SetCrypter(&cfg.CipherInfo)
 	client.SetConcurrency(uint(cfg.Concurrency))
 	client.SetSwitchModeInterval(cfg.SwitchModeInterval)
-	client.InitClients(u, false, false)
+	client.InitClients(ctx, u, false, false)


is the log restore affected by the modified InitClient?

br/pkg/restore/client.go

br/pkg/restore/import.go

ti-chi-bot · 2024-01-03T10:16:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BornChanger, Leavrth

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Leavrth]
~~br/OWNERS~~ [BornChanger,Leavrth]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2024-01-03T10:16:31Z

[LGTM Timeline notifier]

Timeline:

2024-01-03 09:28:24.409406731 +0000 UTC m=+2249195.446633658: ☑️ agreed by Leavrth.
2024-01-03 10:16:30.05398002 +0000 UTC m=+2252081.091206946: ☑️ agreed by BornChanger.

BornChanger · 2024-01-03T12:54:15Z

/retest

ref pingcap#33937

ti-chi-bot bot added do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Nov 2, 2023

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 2, 2023

ti-chi-bot bot added ok-to-test Indicates a PR is ready to be tested. and removed do-not-merge/needs-linked-issue do-not-merge/needs-tests-checked labels Nov 3, 2023

ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 17, 2023

3pointer force-pushed the two_phase_split branch from acf67d6 to 90ed3e5 Compare November 20, 2023 05:59

ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Dec 1, 2023

3pointer marked this pull request as ready for review December 4, 2023 03:57

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 4, 2023

ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 8, 2023

3pointer force-pushed the two_phase_split branch from e4d95c3 to aaa827d Compare December 28, 2023 09:43

Leavrth reviewed Jan 3, 2024

View reviewed changes

Leavrth approved these changes Jan 3, 2024

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 3, 2024

BornChanger approved these changes Jan 3, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 3, 2024

apply patch

9859de1

3pointer force-pushed the two_phase_split branch from 7517755 to 9859de1 Compare January 3, 2024 11:27

ti-chi-bot bot merged commit 44f33c6 into pingcap:master Jan 3, 2024
21 checks passed

AilinKid pushed a commit to AilinKid/tidb that referenced this pull request Jan 17, 2024

restore: refactor split code for two phase split. (pingcap#48244)

ae6703c

ref pingcap#33937

3pointer mentioned this pull request Jan 24, 2024

Tracking issues of restore performance improvement. #50701

Open

26 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

restore: refactor split code for two phase split. #48244

restore: refactor split code for two phase split. #48244

3pointer commented Nov 2, 2023 •

edited

Loading

ti-chi-bot bot commented Nov 2, 2023

tiprow bot commented Nov 2, 2023

3pointer commented Nov 3, 2023

3pointer commented Dec 1, 2023

codecov bot commented Dec 4, 2023 •

edited

Loading

sre-bot commented Dec 18, 2023 •

edited

Loading

Leavrth Jan 3, 2024

Leavrth Jan 3, 2024

Leavrth Jan 3, 2024

Leavrth Jan 3, 2024

ti-chi-bot bot commented Jan 3, 2024

ti-chi-bot bot commented Jan 3, 2024

BornChanger commented Jan 3, 2024

restore: refactor split code for two phase split. #48244

restore: refactor split code for two phase split. #48244

Conversation

3pointer commented Nov 2, 2023 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot bot commented Nov 2, 2023

tiprow bot commented Nov 2, 2023

3pointer commented Nov 3, 2023

3pointer commented Dec 1, 2023

codecov bot commented Dec 4, 2023 • edited Loading

Codecov Report

sre-bot commented Dec 18, 2023 • edited Loading

Leavrth Jan 3, 2024

Choose a reason for hiding this comment

Leavrth Jan 3, 2024

Choose a reason for hiding this comment

Leavrth Jan 3, 2024

Choose a reason for hiding this comment

Leavrth Jan 3, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Jan 3, 2024

ti-chi-bot bot commented Jan 3, 2024

[LGTM Timeline notifier]

BornChanger commented Jan 3, 2024

3pointer commented Nov 2, 2023 •

edited

Loading

codecov bot commented Dec 4, 2023 •

edited

Loading

sre-bot commented Dec 18, 2023 •

edited

Loading